good catch. Only possibly relevant in

, but could return the latest jitted version as opposed to the latest version total. This is probably worth fixing

Does this need synchronization?

How does this work with generic methods? Generic methods can have multiple instantiations.

Hm, the thing is this doesn’t look like it ever worked to distinguish generic instantiations when getting the latest EnC version. I wonder if we even need/use this?

Currently I see FindLatestEncData getting used in CordbObjectValue::GetFunctionHelper but it doesn't have to be. Instead of calling CordbModule::LookupOrCreateFunction(token,encVersion) where encVersion ultimately gets calculated in this call you can instead call CordbModule::LookupOrCreateFunctionLatestVersion(token). That LatestVersion variant of the API already has another independent path to discover the latest (potentially unjitted) ENC version based on Debugger::UpdateFunction sending the new ENC version in updates.

Aside from CordbObjectValue::GetFunctionHelper I only see one other callsite for GetNativeCodeInfo and that one doesn't look at the ENC version information that is returned. You could modify GetNativeCodeInfo to only return exactly what that caller needs (MethodDesc+CodeStartAddr) which will eliminate the only callsite that cared about the pLatestEnCVersion param on LookupEnCVersions(). Then you can remove that parameter and this method.

That sounds good. @jkotas it looks like generics in EnC are handled here, in that edits are made per-method and later applied per-instantiation:

token match should be unnecessary. The code should be enough to identify the method.

This will find the most recently JITed method body. It may not be the one with the highest EnC version. My guess is that you expect this is to find the highest EnC version. Is that right?

Yes, copilot pointed this discrepancy out here #128338 (comment). I think I’ll have to bring the mutation of the linked list up next to SetCurrentEnCVersion

We can refactor to remove the dependency on this method entirely (https://github.com/dotnet/runtime/pull/128338/changes#r3265486372)

Currently I see FindLatestEncData getting used in CordbObjectValue::GetFunctionHelper but it doesn't have to be. Instead of calling CordbModule::LookupOrCreateFunction(token,encVersion) where encVersion ultimately gets calculated in this call you can instead call CordbModule::LookupOrCreateFunctionLatestVersion(token). That LatestVersion variant of the API already has another independent path to discover the latest (potentially unjitted) ENC version based on Debugger::UpdateFunction sending the new ENC version in updates.

Aside from CordbObjectValue::GetFunctionHelper I only see one other callsite for GetNativeCodeInfo and that one doesn't look at the ENC version information that is returned. You could modify GetNativeCodeInfo to only return exactly what that caller needs (MethodDesc+CodeStartAddr) which will eliminate the only callsite that cared about the pLatestEnCVersion param on LookupEnCVersions(). Then you can remove that parameter and this method.

We can refactor to remove the dependency on this method entirely (https://github.com/dotnet/runtime/pull/128338/changes#r3265486372)

I'm worried about the performance overhead of an O(N) linked list search. Although I suspect many uses of ENC are small I'm not confident that all of them are and this method can get called a lot.

Rather than a new linked list, how about we add the ENC version to the method DebugInfo? We could add an ENCVersion as the 7th item in the FAT header. This would have no cost for most methods which use the slim header or 1 nibble per method for those using the FAT header.

If we don't have to keep track of unjitted methods, we could stuff the EnC version into the code header.

I do not think it belongs to code header directly.

I think having in the side-table where it is today or in the debug info as suggested by Noah are reasonable options.

We do have yet another code version scheme for tiered compilation and Rejit. Rejit is very much like EnC. Would it make sense to store the EnC version in the existing structure used by tiered compilation and Rejit? (ILCodeVersionNode, NativeCodeVersionNode, etc.)

Would it make sense to store the EnC version in the existing structure used by tiered compilation and Rejit?

Architecturally yes, it makes sense to have it there. I didn't initially propose that because I worried it might be a more elaborate change, but its where I envisioned ENC would be some day. If it winds up being not too hard to add it there right now thats very nice. I am guessing some of the work along that route would be:

• ENC currently uses independent logic to determine the active version and switch between versions
• ICorDebug is aware of ILCodeVersions but currently they have different treatment than ENC versions. Right now ILCodeVersions generate multiple ICorDebugILCode objects parented under the same ICorDebugFunction whereas ENC expects multiple unique ICorDebugFunctions. Assuming new ENCVersions get represented at runtime as new ILCodeVersionNodes we'd need to teach ICorDebug to distinguish them.

If we are just concerned about performance, a cheap alternative would be to add a lookup map keyed by method token on the module, and have it store a pointer to per-token linked lists of (codeAddr, version) - the same structure used in ILCodeVersions. I am not sure how much work it would be to completely unify EnC and ReJIT, or if maybe there is a middle ground where we add a node but say "actually this is an EnC version".

if maybe there is a middle ground where we add a node but say "actually this is an EnC version".

I think it would be a significant improvement compared to what we have today.

if maybe there is a middle ground where we add a node but say "actually this is an EnC version".

I think it would be a significant improvement compared to what we have today.

I can see us introducing ILCodeVersions for each EnC edit, each of which would get a singular NativeCodeVersion. We would not fold the actual logic of EnC into the logic for CodeVersions, which means that the existing iterators over CodeVersions would have to be updated to skip EnC nodes. We could even fold the APIs I just added on the EnC contract into the CodeVersions contract; it would be iterating the same data with a different filter.

Do you think this makes sense or would we have to go further in integrating the two for it to make sense?

I don't believe finding EnC methods, fields, etc would be easily folded into CodeVersions. So we would still need an EnC contract until this goes away.

Do you think this makes sense or would we have to go further in integrating the two for it to make sense?

It makes sense to me. I understand that ReJIT and EnC are exposed differently in the public APIs, so it is hard to make them handled uniformly.

finding EnC methods, fields, etc would be easily folded into CodeVersions. So we would still need an EnC contract until this goes away.

It is fine for these to be separate.

We would not fold the actual logic of EnC into the logic for CodeVersions, which means that the existing iterators over CodeVersions would have to be updated to skip EnC nodes.

I'm not sure what boundary this implies exactly. Here is my initial thought which may or may not match what you had in mind?

• CodeVersionManager::IsMethodSupported(), MethodDesc::IsVersionable() and IsVersionableWithPrecode() would now return TRUE for editable methods in ENC enabled modules.
• MethodDesc::DetermineAndSetIsEligibleForTieredCompilation() would continue to return FALSE for methods in ENC enabled modules
• ReJIT remains incompatible with ENC enabled modules. ReJitManager::UpdateActiveILVersions would continue to return a failure.
• Each ENC method version would have a corresponding ILCodeVersion. The ILCodeVersion would have a single associated NativeCodeVersion.
• CodeVersionManager::GetILCodeVersions and CodeVersionManager::GetNativeCodeVersions would enumerate the ENC versions. Upstream consumers such as ReJitManager::GetReJITIDs() would filter the enumeration to avoid exposing ENC versions as if they had been created by ReJIT.
• MethodDesc::ResetCodeEntryPointForEnC() would go away. Switching to a new ENC version would use CodeVersionManager::SetActiveILCodeVersions(). Not sure if CodeVersionManager is the place where we'd enforce it, but I'd expect each module-level ENC edit always sets the most recent IL version of the edited methods as active and never reverts back.
• CodeVersionManager::GetActiveILCodeVersion would return the active ENC version of the method, whatever it is.
• We would stop using the Module::SetDynamicIL() mechanism to store ENC IL pointers and store the IL code pointer in the ILCodeVersion instead.

If you investigate and find this would run into issues please do raise it - this was just my initial guess at what might work well.